Skip to content

perf(mempool): collapse queue Push to single write-lock#1069

Draft
daps94 wants to merge 1 commit intomainfrom
perf/queue-single-lock
Draft

perf(mempool): collapse queue Push to single write-lock#1069
daps94 wants to merge 1 commit intomainfrom
perf/queue-single-lock

Conversation

@daps94
Copy link

@daps94 daps94 commented Mar 11, 2026

Summary

  • Collapses Push() from a two-lock pattern (RLock for isFull() → Lock for insert) into a single write-lock with inline capacity check
  • Eliminates a TOCTOU race: under the old pattern, the queue could become full between the RLock check and the Lock acquisition, leading to unbounded growth past maxSize
  • Removes the now-obsolete isFull() helper

Motivation

The original Push() called isFull() which acquired an RLock, released it, then acquired a full Lock for the actual insert. Between the two locks, another goroutine could insert, meaning:

  1. The isFull() check could pass, but by the time Lock is acquired the queue is already full — silently exceeding maxSize
  2. Conversely, isFull() could reject when there was actually space — unnecessary rejections under contention

The fix is a single Lock that checks capacity and inserts atomically.

Benchmark Results (benchstat, 5 runs, Apple M4 Pro)

This change affects the Insert path, not SelectBy directly. However, the iterator benchmark shows indirect improvement from reduced lock contention between the insert goroutine and the background queue drainer:

                                      │   main   │ queue-lock │
                                      │  sec/op  │ sec/op  vs │
EVMMempoolIterator/EVM_2500/Cosmos_0     4958µ     3362µ  -32%
EVMMempoolIterator/EVM_10000/Cosmos_0   20541µ    17910µ  -13%
EVMMempoolIterator/EVM_0/Cosmos_50        97µ       73µ   -24%
EVMMempoolIterator/EVM_0/Cosmos_500      224µ      150µ   -33%
EVMMempoolIterator/EVM_0/Cosmos_2500    1212µ      655µ   -46%
EVMMempoolIterator/EVM_0/Cosmos_5000    2196µ     1462µ   -33%

Geomean latency:  -22%

Files Changed

File Change
mempool/internal/queue/queue.go Rewrite Push() with single Lock; remove isFull()

Test Plan

  • go test -tags=test -race ./mempool/... — 88 tests pass with race detection
  • 5 queue-specific tests pass
  • Correctness: eliminates TOCTOU race on capacity check

🤖 Generated with Claude Code

Remove the separate isFull() RLock check from Push() and perform the
capacity check inside the write lock. This eliminates a TOCTOU window
between the capacity check and the actual push, and reduces lock
round-trips from two to one per insertion.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.10%. Comparing base (a5aa523) to head (dda1ac8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1069      +/-   ##
==========================================
- Coverage   64.11%   64.10%   -0.01%     
==========================================
  Files         331      331              
  Lines       23303    23304       +1     
==========================================
- Hits        14941    14940       -1     
- Misses       6946     6948       +2     
  Partials     1416     1416              
Files with missing lines Coverage Δ
mempool/internal/queue/queue.go 84.41% <100.00%> (-6.38%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant